Fix ignore minimatch to include folders starting with dot #8074
Conversation
|
@amiramw Could you check please why build is failing? |
|
@akosyakov can you help me understand why is it failing? It's not clear to me from the logs. is it stable? I see that it only failed on linux. maybe should try to retrigger? |
for example during npm install there is a temp folder called .staging and we don't want to listen to those files Signed-off-by: Amiram Wingarten <[email protected]>
|
@akosyakov after rebase it seems to pass |
There was a problem hiding this comment.
@amiramw do you perhaps have the code of the extension hosted anywhere (ex: github repository), I wanted to see what it does or expects. I'm not sure what the pull-request is trying to fix, for example when a package.json file under .bin is opened it is being watched:
root INFO [nsfw-watcher: 15227] Started watching: /home/evinfug/workspaces/theia/node_modules/.bin/package.json
root INFO [nsfw-watcher: 15227] Stopped watching: /home/evinfug/workspaces/theia/node_modules/.bin/package.json
What problem is the pull-request solving, it was not clear to me by the description.
|
@vince-fugnitto the git repo for this test extension is https://github.com/amiramw/vscode-file-watch-package-json It basically listen to package.json file changes and print those events to output: const watcher = vscode.workspace.createFileSystemWatcher("**/package.json");
const outputChannel = vscode.window.createOutputChannel('Package.json Events');
watcher.onDidCreate(uri => outputChannel.appendLine('Created ' + uri.toString()));
watcher.onDidChange(uri => outputChannel.appendLine('Changed ' + uri.toString()));nsfw listens to every file change however theia/packages/filesystem/src/node/nsfw-watcher/nsfw-filesystem-watcher.ts Lines 234 to 237 in 7f729f2 The ignored glob pattern is in settings.json and the default value is: "files.watcherExclude": {
"**/.git/objects/**": true,
"**/.git/subtree-cache/**": true,
"**/node_modules/**": true
}My fix is to configure the ignore glob pattern minimatch so that it will also identify folders starting with dot in the path. Otherwise events are fired also for paths inside node_modules which is wrong and incompatible with vs code. The vscode extension I provided test that. |
There was a problem hiding this comment.
@amiramw thank you for taking the time to describe the issue further and the use of dot.
Logically the change makes sense and I verified that the logs are not outputted to the output channel with your updates.
vince-fugnitto
added
the
file-watchers
for example during npm install there is a temp folder called .staging and we don't want to listen to those files.
Signed-off-by: Amiram Wingarten [email protected]
What it does
Add minimatch dot option to make sure also folders starting with dot are excluded from watched. See also https://github.com/isaacs/minimatch#dot
The new behavior complies also with VS Code watch ignore behavior.
How to test
Before this PR there was an output message to output channel "Package.json Events" which is wrong. After this PR there is no such ourput.
Review checklist
Reminder for reviewers